Conversation
There was a problem hiding this comment.
Pull request overview
This PR adds support for discontinued parts in HLS streaming by introducing a part_index field to track segments across discontinuity boundaries. This enables proper handling of streams that have been split into multiple parts due to stream discontinuities.
Key changes:
- Added
part_indexfield toSegmentInfoand related structures to track segment parts - Updated HLS parsers to calculate
part_indexbased on discontinuity markers - Modified merge implementations (proxy, pipe, concat, auto) to handle multi-part outputs
Reviewed changes
Copilot reviewed 13 out of 14 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| flake.lock | Updated nixpkgs and rust-overlay dependencies |
| crates/iori/src/lib.rs | Added part_index() method to StreamingSegment trait with default value of 0 |
| crates/iori/src/segment.rs | Added part_index field to SegmentInfo struct and implemented forwarding in trait implementations |
| crates/iori/src/hls/segment.rs | Added part_index field to M3u8Segment and implemented part_index() method |
| crates/iori/src/hls/source.rs | Populated part_index field when creating segments from playlist |
| crates/iori-hls/src/parse.rs | Implemented logic to track and increment current_part_index based on discontinuity tags |
| crates/iori-hls/src/models.rs | Added part_index field to MediaSegment and discontinuity_sequence to MediaPlaylist |
| crates/iori-hls/src/m3u8_rs.rs | Updated conversion from m3u8_rs types to track part_index based on discontinuity markers |
| crates/iori/src/merge/proxy.rs | Modified playlist generation to insert discontinuity markers based on part_index changes |
| crates/iori/src/merge/pipe.rs | Updated message passing to include part_index and handle part transitions in file output |
| crates/iori/src/merge/concat.rs | Refactored to sort by part_index and create separate output files per part |
| crates/iori/src/merge/auto.rs | Restructured to process segments grouped by part_index and generate multiple output files |
| crates/iori-hls/tests/fixtures/archive_02.m3u8 | Added test fixture for playlist with discontinuity |
| crates/iori-hls/tests/cases.rs | Added test case for archive_02 fixture and renamed existing test |
Comments suppressed due to low confidence (1)
crates/iori/src/merge/proxy.rs:219
- The discontinuity sequence calculation is incorrect. The
EXT-X-DISCONTINUITY-SEQUENCEtag should indicate the discontinuity sequence number of the first media segment in the playlist. However, this code is calculating the total number of discontinuities encountered and inserting it as the discontinuity sequence value.
In HLS, the discontinuity sequence should represent the sequence number of the discontinuity that applies at the start of the playlist. Since discontinuities are being added inline as segments are iterated, the initial discontinuity sequence should be based on the first segment's part_index, not the count of discontinuities. If you're tracking parts with a part_index starting at the discontinuity_sequence value, the first segment's part_index should be used as the discontinuity sequence value.
// Add discontinuity sequence at the beginning if there were any discontinuities
if discontinuity_sequence > 0 {
let header = format!("#EXT-X-DISCONTINUITY-SEQUENCE:{}\n", discontinuity_sequence);
// Insert after version line
let parts: Vec<&str> = playlist.split("#EXT-X-VERSION:3\n").collect();
playlist = format!("{}#EXT-X-VERSION:3\n{}{}", parts[0], header, parts[1]);
}
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
crates/iori/src/merge/concat.rs
Outdated
| if !success { | ||
| output = File::create(namer.next_path()).await?; | ||
| let mut current_part_index: Option<u64> = None; | ||
| let mut output: Option<File> = None; |
There was a problem hiding this comment.
The variable output is declared but never used after initialization. It's set to None at line 101 and never read or written to again in the function. This appears to be dead code that should be removed.
| let mut output: Option<File> = None; |
|
|
||
| prev_sequence = Some(*sequence); | ||
| prev_failed = current_failed; | ||
| prev_part_index = Some(segment.part_index); |
There was a problem hiding this comment.
The discontinuity detection logic updates prev_part_index even when a segment has failed and is skipped from the playlist. This means that if a failed segment causes a part_index change, the discontinuity marker won't be inserted before the next successful segment with a different part_index. The prev_part_index should only be updated when the segment is actually included in the playlist (i.e., when !current_failed).
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 13 out of 14 changed files in this pull request and generated 2 comments.
Comments suppressed due to low confidence (1)
crates/iori/src/merge/proxy.rs:218
- The logic for adding the EXT-X-DISCONTINUITY-SEQUENCE tag is incorrect. According to the HLS specification, EXT-X-DISCONTINUITY-SEQUENCE should indicate the discontinuity sequence number of the first media segment in the playlist, not the total count of discontinuities. The current implementation counts the total number of discontinuities and inserts that count, which would be incorrect if the playlist doesn't start from the beginning. The discontinuity_sequence should track the starting value, not be incremented for each discontinuity encountered.
// Add discontinuity sequence at the beginning if there were any discontinuities
if discontinuity_sequence > 0 {
let header = format!("#EXT-X-DISCONTINUITY-SEQUENCE:{}\n", discontinuity_sequence);
// Insert after version line
let parts: Vec<&str> = playlist.split("#EXT-X-VERSION:3\n").collect();
playlist = format!("{}#EXT-X-VERSION:3\n{}{}", parts[0], header, parts[1]);
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Yesterday17
force-pushed
the
feat/discontinued-parts
branch
from
597b140 to
51a4144
Compare
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
No description provided.